module_adapter: fix memory API debug check for DP modules#10862
Conversation
jsarha
requested review from
LaurentiuM1234,
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
marcinszkudlinski,
mmaka1,
pblaszko,
plbossart and
ranj063
as code owners
There was a problem hiding this comment.
Pull request overview
This PR updates the module adapter’s memory API debug thread check to support DP modules by allowing allocations from either the IPC/resource-manager thread or the DP processing thread, and ensures the module allocation context is zero-initialized.
Changes:
- Add a
dp_threadfield tostruct module_resourcesfor tracking the DP thread ID (Zephyr + memory API debug builds). - Update
MEM_API_CHECK_THREADto accept calls from eitherrsrc_mngrordp_thread. - Switch
mod_alloc_ctxallocation fromrmalloc()torzalloc()to zero-initialize the allocation context.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/schedule/zephyr_dp_schedule_application.c | Records the DP thread ID into module resources for memory API debug checks. |
| src/include/sof/audio/module_adapter/module/generic.h | Extends module_resources with a dp_thread field under Zephyr memory debug. |
| src/audio/module_adapter/module/generic.c | Updates MEM_API_CHECK_THREAD to allow both resource-manager and DP threads. |
| src/audio/module_adapter/module_adapter.c | Uses rzalloc() for mod_alloc_ctx to ensure zero initialization. |
Add dp_thread field to module_resources so that the memory API debug check accepts allocations from both the IPC thread (rsrc_mngr) and the DP thread. Update MEM_API_CHECK_THREAD to check both thread IDs. Also change mod_alloc_ctx allocation from rmalloc to rzalloc to zero-initialize the structure. The code assumes the dp_thread member is NULL in case of a non-DP module. Both threads using the memory tracking features is not a problem since the memory operations are synchronized with semaphores and events so that both threads are never doing resource operations at the same time. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
force-pushed
the
get_fewer_resource_mngr_warnings
branch
from
e408f4a to
c1867b0
Compare
| k_tid_t tid = k_current_get(); \ | ||
| if (((res)->rsrc_mngr != NULL || (res)->dp_thread != NULL) && \ | ||
| (res)->rsrc_mngr != tid && (res)->dp_thread != tid) \ | ||
| LOG_WRN("cur %p != mngr %p or dp %p", tid, (res)->rsrc_mngr, (res)->dp_thread); \ |
There was a problem hiding this comment.
this doesn't quite explain why this is safe... As is described in Kconfig, this code is checking that module adapter memory allocations are happening on a single thread thus confirming the assumption that no locking is needed. This change now states, that allocations are allowed on two threads, but it doesn't verify or explain why that is safe? Why those threads cannot preempt each other and thus corrupt the allocation context? AFAIU this preemption isn't happening because these two threads use the API at distinct time intervals - during pipeline construction, destruction, or at run-time. So maybe instead of adding a second context we need to switch between the two: at certain times it's the IPC thread that's managing the resources, at other times it's the DP thread?
Add dp_thread field to module_resources so that the memory API debug check accepts allocations from both the IPC thread (rsrc_mngr) and the DP thread. Update MEM_API_CHECK_THREAD to check both thread IDs.
Also change mod_alloc_ctx allocation from rmalloc to rzalloc to zero-initialize the structure. The code assumes the dp_thread member is NULL in case of a non-DP module.
Both threads using the memory tracking features is not a problem since the memory operations are synchronized with semaphores and events so that both threads are never doing resource operations at the same time.